Skip to content

Adds job_id as path param in update permission#38962

Merged
Lee-W merged 4 commits into
apache:mainfrom
SubhamSinghal:bugfix/add-path-param-to-update-permission
Apr 14, 2024
Merged

Adds job_id as path param in update permission#38962
Lee-W merged 4 commits into
apache:mainfrom
SubhamSinghal:bugfix/add-path-param-to-update-permission

Conversation

@SubhamSinghal

Copy link
Copy Markdown
Contributor

job_id needs to be passed as path param in update job permission api.
api link: https://docs.databricks.com/api/workspace/jobs/updatepermissions

@pankajkoti pankajkoti left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @SubhamSinghal Were we able to test this with the operator in a DAG that this updates the job's permission well now?

@SubhamSinghal

Copy link
Copy Markdown
Contributor Author

@pankajkoti not till now. I am trying to understand how to test this locally.
I will add try catch in this function so that it will not disrupt original flow

@pankajkoti

pankajkoti commented Apr 12, 2024

Copy link
Copy Markdown
Member

@pankajkoti not till now. I am trying to understand how to test this locally. I will add try catch in this function so that it will not disrupt original flow

@SubhamSinghal Are you not using Breeze for local development? It makes life a lot easier for developing and testing.

@pankajkoti pankajkoti requested a review from Lee-W April 12, 2024 13:55
Comment thread airflow/providers/databricks/hooks/databricks.py Outdated
@SubhamSinghal

Copy link
Copy Markdown
Contributor Author

Tested. Code is working fine.

@Lee-W Lee-W self-requested a review April 13, 2024 16:39
Comment thread airflow/providers/databricks/hooks/databricks.py Outdated
Comment thread airflow/providers/databricks/hooks/databricks.py Outdated
@pankajkoti pankajkoti force-pushed the bugfix/add-path-param-to-update-permission branch from 24479b3 to 331dff8 Compare April 14, 2024 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants